Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: Remove warning raising after new matplotlib release #31573

Merged
merged 16 commits into from
Feb 2, 2020

Conversation

charlesdong1991
Copy link
Member

@charlesdong1991 charlesdong1991 commented Feb 2, 2020

seems CI is broken after the new release of matplotlib

@charlesdong1991
Copy link
Member Author

xref #30588

import matplotlib as mpl

version = mpl.__version__
if version > "3.1.2":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use LooseVersion

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can also just define a new mpl skip decorator and skip if < 3.1.2

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for your quick reply, changed, and let me know if this is what you meant @jreback

@@ -59,16 +60,15 @@ def test_register_by_default(self):
call = [sys.executable, "-c", code]
assert subprocess.check_call(call) == 0

@td.skip_if_low_mpl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes right idea

i think we have some skips that are based on mpl versions already (maybe in the other test plotting files)

can u name like them

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emm, i see we have skip_if_no_mpl and skip_if_mpl decorators, but seems no version involved.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does skip_if_old_mpl sound better? or skip_if_lower_3_1_2_mpl?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latter

@pep8speaks
Copy link

pep8speaks commented Feb 2, 2020

Hello @charlesdong1991! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-02-02 16:12:22 UTC

@alimcmaster1 alimcmaster1 added the CI Continuous Integration label Feb 2, 2020
@@ -91,6 +91,7 @@ def test_matplotlib_formatters(self):
assert Timestamp not in units.registry
assert Timestamp in units.registry

@td.skip_if_lower_3_1_2_mpl
Copy link
Contributor

@jreback jreback Feb 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry we don't really want to add this as a full decorator, just do this

@td.skip_if_no("matplotlib", min_version="3.1.2")

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments, ping on green.

@jreback jreback added this to the 1.0.1 milestone Feb 2, 2020
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great. ping on green.

@charlesdong1991
Copy link
Member Author

charlesdong1991 commented Feb 2, 2020

this looks better simpler, i didn't notice this skip_if_no decorator, thanks for the tip! @jreback

since it uses >= in safe_import, so setting the min_version to 3.1.3, it works locally for me

@charlesdong1991
Copy link
Member Author

ping @jreback

@jreback jreback merged commit 36f7e22 into pandas-dev:master Feb 2, 2020
@jreback
Copy link
Contributor

jreback commented Feb 2, 2020

thanks for pro-actively fixing this @charlesdong1991 !

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Feb 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants